Got segfault in devtools::test(filter = "compile-package") - #84
Got segfault in devtools::test(filter = "compile-package")#84mns-nordicals wants to merge 1 commit into
Conversation
…t in the tests. I don't know if something is wrong with my setup or not. But this fixes it on my end.
|
Strange. Now we get the same error that this was suppose to fix. Locally I don't get the error anymore (well - I don't get them on this PR branch and neither on main after running So if don't know if it is something with my setup. Please close/reject if you see no value here. |
|
Thanks for pulling this together! I do think it's tricky to get testing package-related-workflows within a package right. I remember there was a lot of tricky issues around this in S7 as well. It might be worthwhile to look at how we solved that problem there. There we actually run I don't think that using Ideally, we should be running an actual installation, albeit with some options to make it a a lighter and faster one than normal (the test run time is getting a little too long already). |
|
Closing as I didn't run into the problem again and it was fixed by just re-installing the package. |
Below I provide an ai summary of the fix (codex did this mostly by it self)
Problem
After fetching the latest changes to quickr to my fork and running tests I get two failed tests in
devtools::test(filter = "compile-package")Solution
It turns out that the solution was that the tests were running with an older installed version of
quickrand runningdevtools::install()fixed the problem. I was also surprised to it had passed github automated tests.Nevertheless, before I figured out the problem I got codex to try an investigate the issue. After learning the "real" problem it still thinks it's changes makes the process more robust. So if you think the same you can merge these changes and if you think they are not necessary and we should use the installed version in the package tests then you are free to disregard this PR.
note, I have a local folder called .ai that i added to rbuldignore.
Segfault investigation:
test-compile-package.R(AI summary of changes and fix)Summary
The segfault was caused by the subprocess loading an older installed
quickrinstead of the dev version. The installed version generated aQuickrEntriesarray without a{NULL, NULL, 0}sentinel, which is required byR_registerRoutines(). When the test package’s DLL was loaded,R_registerRoutines()walked past the array and crashed (address 0x1). Ensuring the subprocess loads the dev quickr fixes the crash because the dev version correctly emits the sentinel.After running
devtools::install()on main, the segfault stopped. That indicates the crash was triggered because the subprocesses were loading an installed quickr that was older than the dev sources. The change still improves robustness because it removes the implicit dependency on whatever installed version happens to be on the library path.Environment / Toolchain
From the failing run and checks:
Evidence / Reproduction
pkgload::load_all()in a subprocess while loading the temporary test package DLL.quickr_entrypoints.chad:{NULL, NULL, 0}entry)R_registerRoutines()expects a sentinel-terminated array. Without it, it reads past the array and crashes (segfault at address0x1).Fix Implemented
1) Ensure
compile_package()subprocess loads dev quickrWhen
compile_package()launches a freshRsubprocess, it now preloads the dev quickr (if the current session is a dev package) before callingpkgload::load_all('.'):R/compile-package.R(lines 31–53)pkgload::is_dev_package("quickr")getNamespaceInfo(asNamespace("quickr"), "path")r_codelike:2) Ensure test subprocesses do the same
The tests that use
run_r()now preload the dev quickr before loading the test package:tests/testthat/test-compile-package.R(lines 81, 151–157, 173–179)pkgload::load_all('.')inside the subprocess.Why this fixes it
The dev quickr emits
QuickrEntrieswith the required sentinel:With the sentinel present,
R_registerRoutines()stops safely and the DLL loads without crashing.Verification
R -q -e 'devtools::test_active_file("tests/testthat/test-compile-package.R")'passes.R -q -e 'devtools::test()'passes.R -q -e 'rcmdcheck::rcmdcheck(error_on = "warning")'passes (network warnings only).Files Changed
R/compile-package.Rtests/testthat/test-compile-package.R.Rbuildignore(adds^\.ai$afterair format .)